Hi,

thank you. This looks like an interesting patch (I did not try it yet); I have not been aware of std::deque which seems to be a good match for the given purpose. One "obvious" problem is that we still support compiling DCMTK with C++98 compilers (while using C++11 as the default). We might drop C++98 support in the future but right now we would need to make those guys happy as well.

In the past we wrote our own classes (in ofstd, like OFVector, OFList, OFTuple etc.) as drop-in replacements for std lib features but that's probably too much work for std::deque also before the background that we might not need it any more in a few years. Another way to go is to only use the new behavior if C++11 or greater is available and otherwise fall back to the old implementation.

What do you think?

Thank you for your answer. I think std::deque itself is included in C++98, but some minor parts of the PR (such as std::next and std::prev) are not. However, I think it should be possible to rewrite those parts so that they compile even on C++98.

So, I think two options here:

• Use the new behavior only if C++11 is available, as you suggest. I can do this, if you like.
• Update the new (std::deque based) code so that it should compile even on C++98. I could try this too, but I'm not sure how to test as I don't have a C++98-only compiler easily available I'm afraid.

I just read on cppreference.com: "Insertion or removal of elements - linear O(n)."
This is where the existing double-linked list is probably better (in terms of complexity).

@jriesmeier Insertion or removal at the end is O(1) which is the standard case when reading files.

@michaelonken That's true, but reading DICOM files/datasets is not the only use case. I would rather suggest to check where @reunanen had performance problems with the existing implementation and try to enhance this particular use case.
For example, I can remember that parsing large DICOMDIR files or large Sequence of Items elements was pretty slow before we changed the internal implementation (avoid iterating over an index).

Yes, that makes sense. I can try and see if I could easily change the higher-level implementation to use something that might be better suited for the task perhaps.

Unfortunately I don't have permission to share the example image, but I don't think it's extremely atypical or anything. For what it's worth: I started looking into this whole thing because a client (a hospital) mentioned that reading the exact same image pixels from a DICOM file was some 2x slower than reading in a corresponding Aperio .svs file.

The example file is a whole-slide pathology image that is about 1 gigabyte in size. While the image itself does not really look unusual to me, I'm not sure if the DICOM representation is "typical", at least just yet. (I'm certainly not a DICOM expert.)

I am aware of the DICOM representation of Whole Slide Microscopy Images (WSI). In fact, some of the performance improvements I referred to in my previous comment had been motivated by these huge WSI objects. Processing DICOM SOP Instances with 1 GBytes in size or even more than 100 GBytes in size is no problem at all if you know what you are doing (e.g., I implemented a software for this purpose based on the DCMTK for one of my customers). Iterating over the index of a data structure that is based on DcmList is certainly not a good idea, but I agree that this could (and should?) be improved.

@michaelonken That's true, but reading DICOM files/datasets is not the only use case. I would rather suggest to check where @reunanen had performance problems with the existing implementation and try to enhance this particular use case. For example, I can remember that parsing large DICOMDIR files or large Sequence of Items elements was pretty slow before we changed the internal implementation (avoid iterating over an index).

I know, however, I thought O(n) is also be the complexity for random insertion (or access in general) in DcmList.

I have an idea for an alternative implementation; let me try and see if I can get a similar performance improvement in my use case, without sacrificing the performance of inserting in the middle of the list (which is indeed O(1) at the moment).

I'll get back to you guys when I've been able to try it out.

Closing in favor of #114.